We do have a new devices list command that is merged by not yet released.

Would it make more sense to have a command code42 devices list --matter-id <UID> instead?

Just want to make sure all the options are considered, I don't really understand the use-case.

That's an interesting point. I chose to add it to legal-hold show because that command already showed details on a legal hold matter object, including users on legal hold and (optional) preservation policy details. I thought showing devices on legal hold would be a logical fit here.

From what I gather, the devices command renders mostly the Computer api resource. Unfortunately, there is no flag in the Computer api response that states whether a device is on legal hold. In order to pull legal hold devices, you first need to pull legal hold membership to get a list of users and then find the devices associated with those users. It would not be impossible to move this into the devices command, but we would still have to run some sort of legal hold py42 resource to determine the scope of devices.

Pinging @ceciliastevens for input as well.

Or a code42 legal-hold list-devices [MATTER-ID]. Trying to find a way to afford output format options (At least table and csv).

I agree, if we can find a way to structure this to add a format option, that would be helpful to most customers.

Currently, the legal-hold show command outputs data in the following formats:

• Matter info - table
• Users - columns
• (optional) Preservation policy - JSON

What do you think about removing users from the show command and creating code42 legal-hold list-users [Matter ID] with an --include-devices optiion? I think it makes sense to keep users and devices closely tied together, rather than in separate commands. From a code perspective, to get devices, you'll have to get users first, anyway.

I like that! @timabrmsn What are you thoughts on this? I believe you were the one who implemented the show command.

I actually think it makes more sense to add legal hold status as an option to the devices list command, since that would be a useful option for people who are creating a report of all their devices to know which ones are on legal hold quickly when taking inventory of other things. It could just add a legalHoldMatterId column to the output.

And keeping users under legal-hold show still makes sense to me, since it's a high level overview of the matter, of which membership is a key detail. Otherwise the show command just becomes a single row of the list command with the option to also show preservation policy.

devices list already has --include-usernames which makes a separate call to api/Users and then merges the DataFrames on userId, so adding legal hold status would be easy enough to add using the same logic.

The storage total report is an interesting idea, but might be better suited to include in devices list report also, so it can be used more generically and not tied directly to only legal hold. I'm thinking something like --include-storage-totals on devices list that just adds a column per device for the sum of all that device's archives. Then if someone wanted to get a report broken down by matter Id, they could just filter by matter_id and sum the total in the csv report.

Ok, devices it is. The device pull actually includes userUid without the username option, so it's even easier to merge in the legal hold data.

There are two main things that we want this to accomplish:

• Limit the device list to devices on a specific legal hold matter
  devices --matter-uid - option to limit device search base on a specific legal hold matter, adds in columns such as legalHoldStatus, legalHoldMatterId, legalHoldMatterName

• Add in legal hold membership columns for whatever device query you're running.
  devices -include-legal-hold-membership - option to option to include legal hold membership on to the devices table, adds same columns as above but doesn't limit the population

This feels a little redundant, so any feedback is appreciated.

As for storage-totals: is a sum of all device's archives appropriate? (this is how I wrote my first draft). However, the current legal hold script does some logic to determine which destination is the most recent and complete, and only uses that archive size as the total storage.

I think that since filtering by matter-id outside of the CLI is simple enough (i.e. grep/Select-String or directly in excel), can we leave that to the end-user? I'd like to not have our option list expand too much. And I have some ideas for more generic column/row filtering logic that could work on any cmd that outputs a dataframe. Something like `--filter 'matter-id == 42' that would convert to a pandas expression under the hood. Although I haven't started trying to implement that so I'm not sure how feasible it is...

Also, because a device can be part of multiple legal holds, we need to think of how to display that. Can we just have a single column that has a list of matter-uids the device is part of, maybe separated by semi-colons or something?

Yeah, for storage totals I think adding a usedStorage column that sums the total combined archive size for a device would work, maybe we could also add an archiveCount column that gets added with that option?

Trying to keep what features we add as generic and multi-use-case friendly as possible.

We typically use table format for show commands.

List commands, however, are known to support various output formats, inc. CSV.... My first comment on this PR suggests that this would make sense as a code42 devices list --matter-id <UID> command, which then you can tack on -f CSV or -f JSON etc.... Would that be better?

I find it is better to actually use generators for mocks when they are supposed to be rather than lists.. It saves future headaches, in my experience.

Might need to use side_effect instead of return_value in that case, not sure

I agree with Kiran above that we probably shouldn't use the legal-hold module's internal functions, just for the sake of if someone later needs to change them it could break stuff here.

Since we only need to iterate through all membership dicts, we can simplify getting them all with a generator function instead of building a new list and extending it:

def _get_all_active_hold_memberships(sdk):
    for page in sdk.legalhold.get_all_matters(active=True):
        for matter in page["legalHolds"]:
            for _page in sdk.legalhold.get_all_matter_custodians(
                legal_hold_uid=matter["legalHoldUid"], active=True
            ):
                for membership in _page["legalHoldMemberships"]:
                    yield membership

And pandas has a great function that can 'flatten' nested json into a dataframe for you, which can simplify a lot of this logic:

>>> df = pandas.json_normalize(_get_all_active_hold_memberships(sdk))
>>> df.head()
  legalHoldMembershipUid  active                   creationDate legalHold.legalHoldUid legalHold.name        user.userUid                   user.username                      user.email user.userExtRef
0     955061003769628092    True  2020-05-20T15:58:38.823-05:00     955060991270602172          Test2  945056771151950748                test@example.com                test@example.com            None
1     988814086101244865    True  2021-01-08T11:25:23.645-06:00     946193444244471969           Test  973619347957360307                legalhold@code42.com            legalhold@code42.com        None
2     955320624671252917    True  2020-05-22T10:57:44.903-05:00     946193444244471969           Test  955163198918558214                de60@example.com                de60@example.com            None
3     955320624637698485    True  2020-05-22T10:57:44.892-05:00     946193444244471969           Test  955163197341499910                de59@example.com                de59@example.com            None
4     955320624604144053    True  2020-05-22T10:57:44.885-05:00     946193444244471969           Test  955163195865104902                de58@example.com                de58@example.com            None

json_normalize is great for unpacking the nested dictionary. However, I seem to be losing records (1 per matter). If I run the generator response table through pandas.DataFrame, I get a table with more rows than I do running the same generator response through json_normalize. I haven't quite worked this out yet

Interesting, I'll see if I get the same behavior and try to figure out what's happening too.

I had to serialize the generator into a list in order for all records to be included, which may defeat the purpose of the generator. I pushed those changes moments ago.

Looks like there was a recent pandas bug about the generator thing (they weren't expecting generators to be passed): pandas-dev/pandas#35923

It's fixed in 1.2.2, but I think we can probably just call list() on the result for now instead of force updating to the latest pandas.

Instead of using np.nan for null values here, let's use an empty string, I think NaN would confuse people in the output and think maybe there was a bug. To replace any np.nan values resulting from the .groupby(), you can call .fillna(value="") on the df to replace those as well.

I chose NaN to match the value assumed by pandas of an empty value. NaN's are generated when the legal hold table is merged with the devices table. If I use the above solution prior to merging with the devices table, the legalHoldUid and legalHold.name columns will contain a mix of None's, actual values, and NaNs.

To confirm: which of the following would you prefer?

• NaN's in table view
• None's in table view

(Either will produce null values in the other output formats)

Oops, I meant to say "resulting from the.merge()" instead of .groupby(). Yeah, the .fillna(value="") needs to be called after the merge.

I think we should convert NaN/None values to an empty string for table/CSV outputs, but we'd probably want to keep them as NaN/None when we output to json so it translates to null. But we should push that logic to the DataFrameOutputFormatter so we only need to handle it in one place for any commands that use DataFrames.

I'll start a new PR to rework the formatter to handle that, since I'd like to refactor it a bit for clarity and add some tests around handling dataframe nulls.

NaNs removed

Ok, I've opened #245 to add the null handling in the DataFrameOutputFormatter, so we don't need to be concerned with converting them here, since we want the conversion to be conditional on the output format chosen by the end-user.

Sorry to have you flip/flop this!

Resolved, and awaiting #245

This should be if archive["archiveFormat"] != "ARCHIVE_V2":, since while V3 archives are not prevalent, they still should show up in the results.

can we rephrase printing .... to something like prints the legalhold matter name and ID for any active device on legal hold!?

atleast active device actively seems off while reading.

yes, that is terrible phrasing. Resolved with latest commit

Looks like our changelog has gotten messed up? The #Unreleased tag should not be there...

@unparalleled-js I re-pulled from master. The #Unreleased tag should be at the top, right? Let me know if I should fix it with this PR